Where I'm not entirely sure is cases, where webpack/rollup would pick the pkg.exports.browser.require path. I think those cases, the overrides in pkg.browser must be taken into account by the bundlers in order to generate a browser-compatible build.

Have you tested this case?

Nope, not tested. However, I don't think those fields are related? At least with main, it is 100% ignored if exports is present, I'd assume browser was the same. So older tools without exports support use browser while newer ignores it. Might be wrong, tho 😀

EDIT: Yep, at least webpack behaves in this way: https://webpack.js.org/guides/package-exports/#notes-about-ordering

exports field is preferred over other package entry fields like main, module, browser or custom ones.

Might have misunderstood your question, are you worried about webpack getting the require entry? Not sure why it'd be an issue if webpack picked it? I assume it'll pass the import condition today, in which case it's the same as before (as it was default). If they only pass require that seems correct to not return ESM.

I just ran a quick test: 13ceb79

When using require() webpack picks ./dist/index.js as an entry point and then applies the overrides from pkg.browser, i.e. it uses ./dist/rng-browser.js etc.

I have not verified with rollup yet. But at least for webpack, the pkg.browser field is not ignored, however after all it does the right thing.

I think this should be covered in the browser tests. I can't promise when I can find the time to do this though.

Edit: The test in 13ceb79 is not complete, I added console.log() statements manually in the local ./dist output files to verify that webpack indeed picked ./dist/rng-browser.js etc…

I have not verified with rollup yet. But at least for webpack, the pkg.browser field is not ignored, however after all it does the right thing.

Their docs are wrong, then? Seems weird 😅 But if it works like you want it to, all is well?

Ah, seems like browser is just "aliases", it has nothing directly to do with export conditions. So webpack applies additional mapping after resolving the import: https://webpack.js.org/configuration/resolve/#resolvealiasfields

Ah, seems like browser is just "aliases", it has nothing directly to do with export conditions

Is it even valid to have a "browser" field nested w/in the "exports" block like this? If so, where might we find some sort of description or spec for how it's interpreted?

https://nodejs.org/api/packages.html#nested-conditions

browser I was talking about in that comment is a browserify thing (at the top level), not the export condition I'm adding in this PR

Ah, seems like browser is just "aliases", it has nothing directly to do with export conditions

Is it even valid to have a "browser" field nested w/in the "exports" block like this? If so, where might we find some sort of description or spec for how it's interpreted?

As @SimenB mentioned the pkg.browser field is different from the browser environment in pkg.exports. Indeed, pkg.browser was introduced by browserify a loooong time ago and we've been using it for ages here as well. However, pkg.exports also allows a browser environment which, according to https://webpack.js.org/guides/package-exports/#target-environment seems to be rather widely supported.

However what I don't really like about the current approach in this PR is that we're mixing two concepts: Through pkg.exports.browser.require we're telling bundlers to use the Node.js CommonJS build at ./dist/index.js and then we're relying on the independent, legacy concept of pkg.browser for the browser specific overrides over the Node.js CommonJS build.

I think a cleaner solution would be to provide a consistent CommonJS build for the browser, e.g. in ./dist/commonjs-browser/index.js. Folks have run into similar issues in the past (1, 2, 3, 4) when misconfiguring their bundlers… I would still argue that this is always an unintended misconfiguration and should not be done, folks should not choose the CommonJS builds of their dependencies for browser builds, they should use the ESM build that natively supports tree shaking instead.

However in this case now with jest-environment-jsdom where a browser env is only simulated within a Node.js process things may actually start looking differently. Thinking about this, however, uncovers an interesting aspect: I believe with this current patch and when using jest-environment-jsdom jest may actually end up executing the CommonJS Node.js code and not the browser code. In practice, this will probably produce the intended results for the test cases. However it's still possible that such test cases will pass with Jest but would fail in a browser since browsers will effectively execute different code.

I'm sorry for the long write up, I guess the main message is that this is complicated…

@SimenB do you have an example of where a syntax error is thrown (what you reference in the original PR description)? I would really like to figure out what happens under the hood when jest runs these tests.

A browser build in CJS would be even better of course 🙂

@SimenB do you have an example of where a syntax error is thrown (what you reference in the original PR description)? I would really like to figure out what happens under the hood when jest runs these tests.

Jest provides ['require', 'browser', 'default'] as conditions, meaning default is returned, which is ./dist/esm-browser/index.js.

https://github.com/SimenB/jest-uuid

Remove comment at top of test.js to see it pass (as Jest then runs in node env and provides node condition instead of browser)

@ctavan how do we unblock this? 🙂 This issue is actually stopping me from using the Jest alpha at work 😅 (although I guess I can just patch package.json, but still)

Sorry for the silence and sorry to hear this blocks you.

I'm happy to help unblock this but I have to admit that I'm not sure what the right way forward is. I experimented with building a browser commonjs build, but that only reveals the actual problem…

If you run jest on jsdom.test.js in https://github.com/uuidjs/uuid/tree/simenb-patch/examples/jest-uuid-main you get:

$ ./node_modules/.bin/jest jsdom.test.js
  console.log
    commonjs-browser RNG

      at Object.<anonymous> (../../dist/commonjs-browser/rng.js:3:9)

 FAIL  ./jsdom.test.js
  ● Test suite failed to run

    crypto.getRandomValues() not supported. See https://github.com/uuidjs/uuid#getrandomvalues-not-supported

      20 |
      21 |     if (!getRandomValues) {
    > 22 |       throw new Error('crypto.getRandomValues() not supported. See https://github.com/uuidjs/uuid#getrandomvalues-not-supported');
         |             ^
      23 |     }
      24 |   }
      25 |

      at rng (../../dist/commonjs-browser/rng.js:22:13)
      at Object.v4 (../../dist/commonjs-browser/v4.js:23:32)
      at Object.<anonymous> (jsdom.test.js:5:18)

So the browser commonjs build uses browser APIs (crypto), it has to do this, otherwise we can't sell that as a browser build. However those APIs don't seem to be provided/mocked by jsdom.

I'm currently failing to understand what the expectations are when you run a test case with @jest-environment jsdom? How close to a browser environment is this supposed to be?

If it's not reasonably close, I think that resolving the pkg.exports.browser target for @jest-environment jsdom tests will inevitably cause many problems like the one above, where the code that is offered on the pkg.exports.browser export relies on native browser APIs.

Can you help clarify the expectations?

Edit: So to reiterate: Your patch only "works" because it effectively makes jest execute the node build instead of the browser build.

So the browser commonjs build uses browser APIs (crypto), it has to do this, otherwise we can't sell that as a browser build. However those APIs don't seem to be provided/mocked by jsdom.

That seems perfectly fine to me? "If you want to use uuid in an "old" browser, you need to polyfill". I don't see the issue with that.

I'm currently failing to understand what the expectations are when you run a test case with @jest-environment jsdom? How close to a browser environment is this supposed to be?

As close as possible. If APIs are missing from the env, the module should complain. The fact I want to use CJS instead of ESM is separate from browser vs node. It's not up to this module to say "you cannot use require in the browser", it should just say "here's a browser entry point for CJS" and it's up to the consumer to ensure their environment works for what they're trying to load.

Or uuid can of course continue to say "there is no CJS version for the browser", that's also perfectly valid (and is the situation today, it works as you say by happy coincidence since Jest loads main). 🙂 That just means people using Jest and uuid in a browser needs to find alternatives or mock the dependency, just like with any other dependency. Nothing new or weird about that.

Jest 28 has now been released - which means that Jest and UUID are no longer compatible.

For anyone trying to fix this issue - @dbjorge did a good temp solution here: microsoft/accessibility-insights-web#5421 (comment)

But would be great to have a proper solution to this.

Looping in @dbjorge here since I think this thread is more suitable to discuss the jest/uuid specific issue than #616 (comment).

First of all, thanks for the great summary @dbjorge!

At the risk of repeating what @dbjorge already summarized perfectly well, I'd like to highlight again:

Jest + UUID just "accidentally" worked in the past because Jest used to pick the Node.js build even in environments like jsdom that were supposed to mimic the browser. Apparently this has worked well for folks in practice, so I think a custom jest resolver like this one is actually a great mitigation for the time being as it just puts us back to where we were before Jest 28.

If we were adding a CommonJS browser build for completeness of the support matrix of uuid, we would still need something like jsdom/jsdom#3352 to make this work with Jest 28. And I stand with my statement that I generally think CommonJS for the browser has no production use case. Requiring a whole new build specifically for testing purposes which has otherwise no production use case to me sounds more like something that should be fixed in the testing library rather than in the libraries being tested… So to that end I'd prefer see ESM support land in Jest.

@SimenB two questions:

1. Can you foresee if/when Jest will get ESM support?
2. Assuming, 1. will materialize eventually, could Jest potentially ship fallback resolutions logic for widely spread libraries like uuid that would fall back to resolving the node build (essentially bundling the custom resolver that @dbjorge wrote?)?

I can see how this affects many people, given both jest and uuid are so widespread, I'm just still failing to clearly identify the right solution for this situation.

@ctavan Thanks for the ping!

jestjs/jest#9430 is the meta-issue tracking ESM support in Jest. It is a very long and very soul-crushing read. For the purposes of our discussion here, I think it's probably safe to say that it won't happen soon enough for the folks that need Jest+uuid workarounds to unblock Jest 28 updates.

I want to emphasize that, as a user of both libraries, I don't think anyone on the uuid or Jest sides are making unreasonable decisions here; I don't really feel convinced that a workaround really "belongs" in either library. Particularly:

• I think it's reasonable for the uuid maintainers to push back on adding a CommonJS + browser export just for the sake of one testing framework
• I think the new Jest behavior to try to use browser exports from jest-environment-jsdom is an excellent change overall, and worth breaking this one case for
• If I were a uuid maintainer, I would be very hesitant to touch its exports unnecessarily for fear of breaking backcompat with someone's weird webpack config
• If I were a Jest maintainer, I would be very hesitant to establish a precedent of adding package-specific magic to Jest's default resolver

I think there are probably a large number of users that will be impacted by this, but more importantly than that, I want to recognize that neither Jest nor uuid have any obligation to be cross-compatible with one another. I am a thankful user of the work you have both made available for free, and neither project's maintainers owe me (and certainly not my employer) a damn thing; if I have to maintain a few lines of resolver.js to use them together, that's perfectly fine.

Hey! 👋

@SimenB two questions:

1. Can you foresee if/when Jest will get ESM support?

There is support today, however it's experimental (docs). And given the complete lack of progress on the V8/Node side of things (nodejs/node#37648, AFAIK latest is the (seemingly) stalled https://chromium-review.googlesource.com/c/v8/v8/+/3172764) it will stay experimental (as in you need to run node with --experimental-vm-modules) for the foreseeable future, especially for e.g. node 14 which will likely never be unflagged at this point. So we're realistically talking at least a year (but since I would have said that a year ago... well 😅).

2. Assuming, 1. will materialize eventually, could Jest potentially ship fallback resolutions logic for widely spread libraries like uuid that would fall back to resolving the node build (essentially bundling the custom resolver that @dbjorge wrote?)?

I don't think we'll be adding workarounds for any libraries directly in Jest. The easiest solution would probably be for somebody to create a jest-uuid that people can stick in the moduleNameMapper config. It could just do module.exports = require('uuid') and possibly be a cleaner solution than custom resolvers (as it composes better if people already use a custom resolver). Or potentially having some well maintained custom resolver people can plug in that looks like the one @dbjorge has built, but supporting more modules than just uuid. But the out of the box experience is unlikely to attempt to work around this issue.

I don't think anyone on the uuid or Jest sides are making unreasonable decisions here; I don't really feel convinced that a workaround really "belongs" in either library.

I 100% agree with this statement, well put!

Thanks for chiming in @SimenB!

I don't think we'll be adding workarounds for any libraries directly in Jest. The easiest solution would probably be for somebody to create a jest-uuid that people can stick in the moduleNameMapper config. It could just do module.exports = require('uuid') and possibly be a cleaner solution than custom resolvers (as it composes better if people already use a custom resolver).

@dbjorge what do you think about this approach? Would this be something you'd consider to test-drive on accessibility-insights-web?

Or potentially having some well maintained custom resolver people can plug in that looks like the one @dbjorge has built, but supporting more modules than just uuid.

Or maybe rather this second approach?

Given I currently don't have my own use case for this, I don't think I'll be able to provide a solution (like the jest-uuid wrapper), but I'd be happy to contribute (e.g. I'd be open to host such a module in the uuidjs github & npm org).

For anyone coming here looking for a quick workaround, i did the following in my jest config (which i think is more or less @SimenB's suggestion with a jest-uuid)

moduleNameMapper: {
  "^uuid$": "<rootDir>/node_modules/uuid/dist/index.js",
},

Yep! If your config is in JS, I'd recommend "^uuid$": require.resolve("uuid") instead of hardcoding a path into node_modules, but that should work fine 🙂

The easiest solution would probably be for somebody to create a jest-uuid that people can stick in the moduleNameMapper config. It could just do module.exports = require('uuid') and possibly be a cleaner solution than custom resolvers (as it composes better if people already use a custom resolver).

@dbjorge what do you think about this approach? Would this be something you'd consider to test-drive on accessibility-insights-web?

I actually tried basically this solution before I went with the custom resolver - I don't think a wrapper package is necessary, you can just do something like:

moduleNameMapper: {
  // This forces Jest/jest-environment-jsdom to use a Node+CommonJS version of uuid, not a Browser+ESM one
  // See https://github.com/uuidjs/uuid/pull/616
  //
  // WARNING: if your dependency tree has multiple paths leading to uuid, this will force all of them to resolve to
  // whichever one happens to be hoisted to your root node_modules folder. This makes it much more dangerous
  // to consume future uuid upgrades. Consider using a custom resolver instead of moduleNameMapper.
  '^uuid$': require.resolve('uuid'),
}

However, I rejected this in favor of the custom resolver because my understanding of the moduleNameMapper option is that it would essentially force every transitive dependency on uuid in my project to resolve to whichever version of uuid happens to be hoisted to the root node_modules folder. I'm already working in a project which has multiple versions of uuid in its dependency tree, but even if I wasn't, this scenario creates a maintenance time bomb if uuid ever releases a v9 with breaking changes. I preferred the custom resolver because it lets each dependency chain continue to use (the CommonJS version of) whatever uuid it was already using.

edit: beaten twice, oops

@dbjorge your analysis is correct though, this forces a single version across the entire tree, so might not be correct for all use cases. a resolver based solution is safer in that regard 🙂

Sorry for the silence and sorry to hear this blocks you.

I'm happy to help unblock this but I have to admit that I'm not sure what the right way forward is. I experimented with building a browser commonjs build, but that only reveals the actual problem…

If you run jest on jsdom.test.js in https://github.com/uuidjs/uuid/tree/simenb-patch/examples/jest-uuid-main you get:

$ ./node_modules/.bin/jest jsdom.test.js
  console.log
    commonjs-browser RNG

      at Object.<anonymous> (../../dist/commonjs-browser/rng.js:3:9)

 FAIL  ./jsdom.test.js
  ● Test suite failed to run

    crypto.getRandomValues() not supported. See https://github.com/uuidjs/uuid#getrandomvalues-not-supported

      20 |
      21 |     if (!getRandomValues) {
    > 22 |       throw new Error('crypto.getRandomValues() not supported. See https://github.com/uuidjs/uuid#getrandomvalues-not-supported');
         |             ^
      23 |     }
      24 |   }
      25 |

      at rng (../../dist/commonjs-browser/rng.js:22:13)
      at Object.v4 (../../dist/commonjs-browser/v4.js:23:32)
      at Object.<anonymous> (jsdom.test.js:5:18)

So the browser commonjs build uses browser APIs (crypto), it has to do this, otherwise we can't sell that as a browser build. However those APIs don't seem to be provided/mocked by jsdom.

I'm currently failing to understand what the expectations are when you run a test case with @jest-environment jsdom? How close to a browser environment is this supposed to be?

JSDOM v20 supports crypto.getRandomValues(), fwiw

JSDOM v20 supports crypto.getRandomValues(), fwiw

@SimenB I'm being lazy here (headed out the door to walk dog, get my day started), but does that mean things should "just work" now? ... or that work is still needed w/in this module to resolve this issue?

It means the browser version can be ported to CJS (so just module format change, no logic change needed (iirc))

I can confirm that this now works with jest-environment-jsdom@29.0.0-alpha.0 (which has jsdom@20.0.0 as a dependency).

I have released uuid@9.0.0-beta.0 that contains this patch. Please try it out and let me know if it fixes the Jest interoperability (to be tested with jest@29.0.0-alpha.1).

I have released uuid@9.0.0-beta.0 that contains this patch. Please try it out and let me know if it fixes the Jest interoperability (to be tested with jest@29.0.0-alpha.1).

I just gave it a try and it seems to fix the issue on my side. Looking forward for the stable release(s).

Thanks for your work 💪

@ctavan can confirm it allows us to remove the moduleMapper config at work as well 👍

FWIW, Jest 29 is released (which means jest-environment-jsdom@29 is out, which comes with jsdom@20)

@SimenB @ctavan, When can we expect the release?

I have no idea, Jest 29 has been out for a week

FWIW, Jest 29 is released (which means jest-environment-jsdom@29 is out, which comes with jsdom@20)

Thanks for the info. Awaiting uuid v9 now then :)

uuid@9.0.0 has now been released to npm. Please create a new bug in case this doesn't fix your issue.